implement resources API part 2#17749
Conversation
…ersistentvolume list view (to be deleted)
richard-cox
left a comment
There was a problem hiding this comment.
There's lots of compile / transpile time TS warnings
WARNING Compiled with 13 warnings
warning in ./shell/apis/intf/resources.ts
export 'ResourceType' (reexported as 'ResourceType') was not found in '@shell/apis/intf/resources-api/resource-base' (module has no exports)
| | :--- | :--- | | ||
| | [Cluster API](./resources-api/interfaces/ClusterApi) | Interact with cluster-scoped Kubernetes resources (Pods, Deployments, Services, etc.) | | ||
| | [Management API](./resources-api/interfaces/MgmtApi) | Interact with global Rancher resources (Users, Clusters, Settings, etc.) | | ||
| | [Resource Instance API](./resources-api/interfaces/ResourceInstanceApi) | Interact any given Kubernetes resource instance | |
There was a problem hiding this comment.
i don't think we need to surface this as a major api, it's more a facet of the above two
| sort: ['provisioner'], | ||
| }; | ||
|
|
||
| export const PERSISTENT_VOLUME_CAPACITY = { |
There was a problem hiding this comment.
just in case, remember to remove this with the other persistentvolume changes
| * @interface | ||
| * Data object for creating a new resource. Must include a `type` property. | ||
| * | ||
| * @example |
There was a problem hiding this comment.
probably worth creating an instance of CreateResourceData first and then using it for it's indented purpose. this makes it nice and clear
| * }); | ||
| * ``` | ||
| */ | ||
| export type CreateResourceData = { type: ResourceType } & Record<string, any>; |
There was a problem hiding this comment.
this might be more of a preference thing, but why not just... ?
export type CreateResourceData = {
[key: string]: any,
type: ResourceType,
};
| await model.save(); | ||
|
|
||
| return new ResourceInstanceImpl(model) as T; | ||
| } catch (e: unknown) { |
There was a problem hiding this comment.
i was playing around with the API and got this consoe.error
Resource API error - cluster - Failed to create resource of type "pod": undefined
no stack trace or anything to go on
this took me down a long path of trying to work out how to resolve
- the Error thrown wasn't an error, but an array of strings (this shouldn't happen, but is)
- the console.error didn't print this because an array has not e.message
- i'd try/catch'd the line that threw the error so i didn't get anything on console regarding the error that was actually an array
i think to help with this we just need to update surfaceError
private surfaceError(message: string, e?: any): never {
console.error(`Resource API error - ${ this.storeType } - ${ message }`, e); // eslint-disable-line no-console
throw new Error(`Resource API error - ${ this.storeType } - ${ message }`, { cause: e });
}
| await model.save(); | ||
|
|
||
| return new ResourceInstanceImpl(model) as T; | ||
| } catch (e: unknown) { |
There was a problem hiding this comment.
Following from https://github.com/rancher/dashboard/pull/17749/changes#r3311237610
the object used in create was
{
type: 'pod',
spec: { template: {} }
}
which is missing required stuff that our validation picks up. however the error message is orientated to form world "Name" is required.
for api use though we want the raw path in the error message not the human form name... alternatively we don't run validation and let the http error handle it
| throw new Error(`ResourceInstance API error - ${ this._model?.type }/${ this._model?.id } - ${ message }`, { cause: e }); | ||
| } | ||
|
|
||
| constructor(model: any) { |
There was a problem hiding this comment.
This process is a bit scary and generally with this stuff there's hidden dragons that only surface later in production.
I asked our dear friend gemini and they came up with some good catches
- Object.getOwnPropertyDescriptors(model) only grabs the keys that exist at the exact moment of instantiation. If the underlying _model later receives a new property dynamically (e.g., a websocket update injects a new field that wasn't there initially), the ResourceInstanceImpl instance will not have a getter/setter for it and the property will be invisible to the consumer.
- getOwnPropertyDescriptors only looks at the object's own properties. If the underlying model relies on getters or properties located on its prototype chain (which is common in class-based models like SteveModel), this constructor completely misses them.
- Heavy Instantiation: Looping through every single key and calling Object.defineProperty is computationally expensive.
- Memory Bloat: Inside the loop, get: () => this._model[key] creates a new anonymous closure function for every single property on every single instance.
i think we need to go down the worse but safest path of
- aways return objects returned from dashboard actions without passing them through another class
- when returning the object instance
asit as the required interface - update SteveModel to implement the required interface (note though it's not TS)
though am open to other ideas, but basically we need to ensure we return the instance from the store
| this.surfaceError('Cannot patch: permission denied'); | ||
| } | ||
|
|
||
| const url = this._model.linkFor('update') || this._model.linkFor('self'); |
There was a problem hiding this comment.
we should just be able to call save with additional args
| }); | ||
|
|
||
| if (Array.isArray(response)) { | ||
| return response.map((r: any) => new ResourceInstanceImpl(r)) as FindFilteredPageResponse<T>; |
There was a problem hiding this comment.
we're de-references the collection. so any new / removed entries won't be applied to the response
There was a problem hiding this comment.
same applies for the other map's
Co-authored-by: Richard Cox <18697775+richard-cox@users.noreply.github.com>
Co-authored-by: Richard Cox <18697775+richard-cox@users.noreply.github.com>
Co-authored-by: Richard Cox <18697775+richard-cox@users.noreply.github.com>
Summary
Fixes #16898
Implements Resources API Part 2: create, update, patch, and delete operations at both the
ResourcesApilevel (resources.cluster.*/resources.mgmt.*) and theResourceInstanceApilevel (resourceInstance.*).NOTE: demo interaction with
persistentvolumelist view to test (to be deleted in the end)Occurred changes and/or fixed issues
New write operations added to the Resources API:
resources.cluster.create(data)canCreateresources.cluster.patch(type, id, data)resources.cluster.update(type, id, data)resources.cluster.delete(type, id)resourceInstance.patch(data)canEditresourceInstance.update()canEditresourceInstance.delete()canDeleteAll methods are available on both
clusterandmgmtscopes (ClusterApiandMgmtApiare type aliases toResourcesApi).Existing read operations (
find,findAll,findFiltered) now wrap results inResourceInstanceImpl, so every resource returned by the API haspatch(),update(), anddelete()available out of the box.Technical notes summary
ResourceInstanceImplwrapping: All resources returned from the API (find, findAll, findFiltered, create) are now wrapped inResourceInstanceImpl. This provides the instance-level methods (patch, update, delete) on every returned resource._modelis non-enumerable: The underlying store model reference is stored viaObject.definePropertywithenumerable: false. This preventsstructuredCloneandJSON.stringifyfrom attempting to serialize store internals (functions, dispatch refs, circular references), which is critical for Vue 3 reactive compatibility.declarekeyword for_model: Usesdeclare _model: anyinstead of a class field initializer to avoid TypeScript emitting a runtime assignment that would override the non-enumerableObject.definePropertyset in the constructor.invalidatePageCache: false: TheresourceInstance.patch()method explicitly passesinvalidatePageCache: falsewhen dispatchingloadafter a successful patch. Without this, the store mutation defaultsinvalidatePageCachetotrue, which clears paginated list data and causes resources to disappear from list views.toJSONfallback:ResourceInstanceImpl.toJSON()defensively checks for_model.toJSONbefore calling it, falling back to a shallow spread. This prevents errors when the model lacks atoJSONmethod (e.g. in test mocks).resourceUrlhelper: A new private method onResourcesApiClassImplbuilds resource URLs fromschema.linkFor('collection') + '/' + resourceId. It reuses the existingisNamespaced()validation to enforcenamespace/nameformat for namespaced resources.CreateResourceDatatype: New type exported from the barrel —{ type: ResourceType } & Record<string, any>— enforces the mandatorytypeproperty on create data.delete()added alongsideremove():remove()is kept for backward compatibility.delete()is the new public method that adds RBAC checking before delegating tomodel.remove().Areas or cases that should be tested
resources.cluster.create()with valid data, verify permission denial whencanCreateis falseresources.cluster.patch()sends only partial data withapplication/merge-patch+jsoncontent type, verify namespace validationresourceInstance.patch()checkscanEdit, sends partial data, merges response back into the instance, and does not invalidate paginated list viewsresources.cluster.update()runscleanForSaveand sends full data with PUTresourceInstance.update()checkscanEditand delegates tomodel.save()resources.cluster.delete()sends DELETE request with correct URLresourceInstance.delete()checkscanDeleteand delegates tomodel.remove()resources.mgmt.*Areas which could experience regressions
findFiltered/findPage): TheResourceInstanceImplwrapping infindFilterednow maps over both array results and transient.dataarrays. Verify paginated lists still render correctly.find/findAllconsumers: Any code that usestoStrictEqualorinstanceofchecks against raw store models will seeResourceInstanceImplinstances instead. Property access is unchanged since getters proxy through to the model.ResourceInstanceApi: The interface now exposespatch(),update(), anddelete()— no breaking changes, but extensions get new methods in IntelliSense._modelnon-enumerable change should be transparent, but verify that resource data remains reactive when stored in Vue component state.Screenshot/Video
N/A — API-only changes with no UI modifications.
Checklist
Admin,Standard UserandUser Base